[MTE-5149] - delete auto tests #30
Conversation
WalkthroughThis PR removes skipped test entries from multiple test plans, deletes two test methods (testCopyLink and testSetCustomURLAsHome), and promotes internal ClipBoardTests methods to public visibility. Additional changes include correcting a UI element reference in DisplaySettingsTests and adjusting navigation sequencing in HomePageSettingsUITest. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@firefox-ios/firefox-ios-tests/Tests/XCUITests/ClipBoardTests.swift`:
- Line 11: Remove the unnecessary public visibility on the test helper methods
in ClipBoardTests by changing the declarations of public func checkUrl(), public
func copyUrl(), and public func checkCopiedUrl() to the default internal
visibility (i.e., remove the `public` keyword) so they remain internal to the
ClipBoardTests class.
In `@firefox-ios/firefox-ios-tests/Tests/XCUITests/DisplaySettingsTests.swift`:
- Around line 46-54: In testCheckSystemThemeChanges, after invoking
navigator.performAction(Action.SelectLightTheme) the assertion is checking the
wrong accessibility identifier (darkThemeView); update the assertion to read the
value from AccessibilityIdentifiers.Settings.Appearance.lightThemeView instead
so testCheckSystemThemeChanges validates the light-theme button state correctly.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
firefox-ios/firefox-ios-tests/Tests/AccessibilityTestPlan.xctestplanfirefox-ios/firefox-ios-tests/Tests/FullFunctionalTestPlan.xctestplanfirefox-ios/firefox-ios-tests/Tests/Smoketest.xctestplanfirefox-ios/firefox-ios-tests/Tests/TAESmokeTestPlan.xctestplanfirefox-ios/firefox-ios-tests/Tests/XCUITests/ClipBoardTests.swiftfirefox-ios/firefox-ios-tests/Tests/XCUITests/DisplaySettingsTests.swiftfirefox-ios/firefox-ios-tests/Tests/XCUITests/HomePageSettingsUITest.swift
💤 Files with no reviewable changes (4)
- firefox-ios/firefox-ios-tests/Tests/Smoketest.xctestplan
- firefox-ios/firefox-ios-tests/Tests/FullFunctionalTestPlan.xctestplan
- firefox-ios/firefox-ios-tests/Tests/AccessibilityTestPlan.xctestplan
- firefox-ios/firefox-ios-tests/Tests/TAESmokeTestPlan.xctestplan
🧰 Additional context used
🧬 Code graph analysis (1)
firefox-ios/firefox-ios-tests/Tests/XCUITests/ClipBoardTests.swift (1)
firefox-ios/firefox-ios-tests/Tests/XCUITests/BaseTestCase.swift (1)
mozWaitForValueContains(210-222)
🔇 Additional comments (2)
firefox-ios/firefox-ios-tests/Tests/XCUITests/HomePageSettingsUITest.swift (1)
155-156: LGTM — navigation sequencing makes sense.firefox-ios/firefox-ios-tests/Tests/XCUITests/ClipBoardTests.swift (1)
66-88: Confirm intentional removal ofcheckCopiedUrl()fromtestClipboard().According to the summary,
checkCopiedUrl()was previously called withintestClipboard()but has been removed. The test now relies on Line 86'smozWaitForValueContains(urlBarAddress, value: "http://www.example.com/")to indirectly validate that the paste operation succeeded.This indirect validation is reasonable since it confirms the clipboard content was correctly pasted. However, ensure this change is intentional and that direct clipboard verification (
checkCopiedUrl()) is not required for test coverage.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
|
|
||
| // Check for test url in the browser | ||
| func checkUrl() { | ||
| public func checkUrl() { |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for usages of these methods outside of ClipBoardTests.swift
echo "=== Searching for checkUrl() usage ==="
rg -n --type swift '\bcheckUrl\s*\(' -g '!*ClipBoardTests.swift'
echo -e "\n=== Searching for copyUrl() usage ==="
rg -n --type swift '\bcopyUrl\s*\(' -g '!*ClipBoardTests.swift'
echo -e "\n=== Searching for checkCopiedUrl() usage ==="
rg -n --type swift '\bcheckCopiedUrl\s*\(' -g '!*ClipBoardTests.swift'Repository: qodo-benchmark/firefox-ios-combined-coderabbit
Length of output: 213
Remove public visibility from helper methods—they are not used outside this class.
These three methods (checkUrl(), copyUrl(), checkCopiedUrl()) should remain internal. They are only used within ClipBoardTests and have no external usages across the codebase. Making them public unnecessarily exposes internal test helpers and violates encapsulation. Revert them to the default internal visibility.
🤖 Prompt for AI Agents
In `@firefox-ios/firefox-ios-tests/Tests/XCUITests/ClipBoardTests.swift` at line
11, Remove the unnecessary public visibility on the test helper methods in
ClipBoardTests by changing the declarations of public func checkUrl(), public
func copyUrl(), and public func checkCopiedUrl() to the default internal
visibility (i.e., remove the `public` keyword) so they remain internal to the
ClipBoardTests class.
| // https://mozilla.testrail.io/index.php?/cases/view/3298823 | ||
| func testCheckSystemThemeChanges() { | ||
| navigator.nowAt(NewTabScreen) | ||
| navigator.goto(DisplaySettings) | ||
|
|
||
| // Select Light mode | ||
| navigator.performAction(Action.SelectLightTheme) | ||
| let lightIsSelected = app.buttons[AccessibilityIdentifiers.Settings.Appearance.lightThemeView].value | ||
| let lightIsSelected = app.buttons[AccessibilityIdentifiers.Settings.Appearance.darkThemeView].value | ||
| XCTAssertEqual(lightIsSelected as? String, "1") |
There was a problem hiding this comment.
Fix the light-theme assertion target.
After selecting Light mode, the assertion checks darkThemeView, which likely makes the test fail or pass incorrectly. It should validate lightThemeView.
🛠️ Suggested fix
- let lightIsSelected = app.buttons[AccessibilityIdentifiers.Settings.Appearance.darkThemeView].value
+ let lightIsSelected = app.buttons[AccessibilityIdentifiers.Settings.Appearance.lightThemeView].value📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // https://mozilla.testrail.io/index.php?/cases/view/3298823 | |
| func testCheckSystemThemeChanges() { | |
| navigator.nowAt(NewTabScreen) | |
| navigator.goto(DisplaySettings) | |
| // Select Light mode | |
| navigator.performAction(Action.SelectLightTheme) | |
| let lightIsSelected = app.buttons[AccessibilityIdentifiers.Settings.Appearance.lightThemeView].value | |
| let lightIsSelected = app.buttons[AccessibilityIdentifiers.Settings.Appearance.darkThemeView].value | |
| XCTAssertEqual(lightIsSelected as? String, "1") | |
| // https://mozilla.testrail.io/index.php?/cases/view/3298823 | |
| func testCheckSystemThemeChanges() { | |
| navigator.nowAt(NewTabScreen) | |
| navigator.goto(DisplaySettings) | |
| // Select Light mode | |
| navigator.performAction(Action.SelectLightTheme) | |
| let lightIsSelected = app.buttons[AccessibilityIdentifiers.Settings.Appearance.lightThemeView].value | |
| XCTAssertEqual(lightIsSelected as? String, "1") |
🤖 Prompt for AI Agents
In `@firefox-ios/firefox-ios-tests/Tests/XCUITests/DisplaySettingsTests.swift`
around lines 46 - 54, In testCheckSystemThemeChanges, after invoking
navigator.performAction(Action.SelectLightTheme) the assertion is checking the
wrong accessibility identifier (darkThemeView); update the assertion to read the
value from AccessibilityIdentifiers.Settings.Appearance.lightThemeView instead
so testCheckSystemThemeChanges validates the light-theme button state correctly.
Benchmark PR from agentic-review-benchmarks#4
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.